Skip to content

Comments

feat: passthrough fallback when Clap parse fails + review fixes#200

Open
guillaumedeslandes wants to merge 6 commits intortk-ai:masterfrom
guillaumedeslandes:feat/passthrough-fallback
Open

feat: passthrough fallback when Clap parse fails + review fixes#200
guillaumedeslandes wants to merge 6 commits intortk-ai:masterfrom
guillaumedeslandes:feat/passthrough-fallback

Conversation

@guillaumedeslandes
Copy link
Contributor

@guillaumedeslandes guillaumedeslandes commented Feb 18, 2026 •
edited
Loading

Summary

  • Graceful fallback: when Clap can't parse a command (e.g. git -C /path status), RTK executes the raw command instead of erroring out
  • Guard RTK meta-commands (gain, discover, learn, init, config, proxy, hook-audit, cc-economics) from falling through to raw $PATH execution — bad flags like rtk gain --badtypo show the Clap error directly
  • Fix timer placement so fallback commands capture actual execution time in rtk gain --history (was showing ~0ms)
  • Add 90-day retention cleanup to record_parse_failure() (parse_failures table was never cleaned up)
  • Strip ANSI codes from Clap errors before storing in SQLite
  • Fix smoke test: cargo test assertion now matches RTK's filtered output, fix undefined skip → skip_test calls

Test plan

  • 425 unit tests pass (cargo test)
  • 90/90 smoke tests pass, 0 failed (bash scripts/test-all.sh)
  • rtk gain --badtypo shows Clap error (not "command not found")
  • rtk git -C /path status falls back to raw git -C /path status
  • rtk --help and rtk --version still work normally

Fixes #204, #228, #229

🤖 Generated with Claude Code

@pszymkowiak
Copy link
Collaborator

pszymkowiak commented Feb 20, 2026

Tested locally — solid approach, needs rebase + 2 fixes before merge.

Testing results:

  • 387/387 unit tests pass
  • 82/83 smoke tests pass (1 fail due to branch being behind master)
  • Manual testing confirms fallback works correctly for git -C, git --no-pager, grep -E
  • Normal RTK-filtered commands (git status, git log, --help, --version) unaffected

Must fix:

  1. Rebase on master — branch is 2 commits behind (missing v0.22.0/0.22.1), cargo test count mismatch
  2. Guard RTK meta-commands from fallback — rtk gain --badtypo currently tries to execute a gain binary from $PATH before showing the Clap error. Add a check for known RTK-only subcommands
    (gain, discover, learn, init, config, proxy, hook-audit, cc-economics) and show the Clap error directly for those.

Nice to have (non-blocking):

  • Timer starts after command completes — all fallback commands show ~0ms in rtk gain --history. Move TimedExecution::start() before the .status() call.
  • parse_failures table never gets cleaned up (only record() calls cleanup_old(), not record_parse_failure())
  • Strip ANSI codes from Clap error before storing in SQLite (utils::strip_ansi already exists)

This will fix #204, #228, #229 and any future unknown-flag issues globally. Great work!

@Shankhara Shankhara mentioned this pull request Feb 20, 2026
Open
guillaumedeslandes and others added 5 commits February 23, 2026 14:49
@guillaumedeslandes @claude
feat: passthrough fallback when Clap parse fails
aa6b3ab 
When RTK cannot parse a command (e.g. `rtk git -C /path status`),
instead of exiting with error code 2, it now falls back to running
the raw command directly. This keeps developer workflows unbroken.

- Replace Cli::parse() with try_parse() + run_fallback()
- Add parse_failures SQLite table for failure analytics
- Add `rtk gain --failures` / `-F` to view failure log
- Fallback preserves stdin/stdout/stderr via Stdio::inherit()
- --help and --version still work normally

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@guillaumedeslandes @claude
fix: guard RTK meta-commands from fallback to raw execution
c3f5043 
When Clap fails to parse a meta-command like `rtk gain --badtypo`,
show the Clap error directly instead of trying to execute `gain`
as a binary from $PATH. Adds RTK_META_COMMANDS constant listing
gain, discover, learn, init, config, proxy, hook-audit, cc-economics.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@guillaumedeslandes @claude
fix: move timer start before command execution in fallback
30f24b8 
TimedExecution::start() was called after the command finished,
so all fallback commands showed ~0ms in rtk gain --history.
Now the timer captures actual command runtime.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@guillaumedeslandes @claude
fix: add cleanup_old() call to record_parse_failure()
b1024e9 
The parse_failures table was never cleaned up because only record()
called cleanup_old(). Now parse failures also trigger 90-day retention
cleanup.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@guillaumedeslandes @claude
fix: strip ANSI codes from Clap error before SQLite storage
8142173 
Clap errors may contain terminal color codes. Strip them with
utils::strip_ansi() before storing in the parse_failures table
to avoid garbled output and wasted space.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@guillaumedeslandes @claude
fix: smoke test cargo test assertion and missing skip_test calls
c9816a5 
- cargo test check now matches RTK's filtered output format ("passed")
  in addition to raw "test result:" and "FAILURES"
- Fix undefined `skip` calls to use existing `skip_test` function
  with proper (name, reason) arguments

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@guillaumedeslandes guillaumedeslandes changed the title feat: passthrough fallback when Clap parse fails feat: passthrough fallback when Clap parse fails + review fixes Feb 23, 2026
@guillaumedeslandes
Copy link
Contributor Author

guillaumedeslandes commented Feb 23, 2026

Tested locally — solid approach, needs rebase + 2 fixes before merge.

Testing results:

  • 387/387 unit tests pass
  • 82/83 smoke tests pass (1 fail due to branch being behind master)
  • Manual testing confirms fallback works correctly for git -C, git --no-pager, grep -E
  • Normal RTK-filtered commands (git status, git log, --help, --version) unaffected

Must fix:

  1. Rebase on master — branch is 2 commits behind (missing v0.22.0/0.22.1), cargo test count mismatch
  2. Guard RTK meta-commands from fallback — rtk gain --badtypo currently tries to execute a gain binary from $PATH before showing the Clap error. Add a check for known RTK-only subcommands
    (gain, discover, learn, init, config, proxy, hook-audit, cc-economics) and show the Clap error directly for those.

Nice to have (non-blocking):

  • Timer starts after command completes — all fallback commands show ~0ms in rtk gain --history. Move TimedExecution::start() before the .status() call.
  • parse_failures table never gets cleaned up (only record() calls cleanup_old(), not record_parse_failure())
  • Strip ANSI codes from Clap error before storing in SQLite (utils::strip_ansi already exists)

This will fix #204, #228, #229 and any future unknown-flag issues globally. Great work!

@pszymkowiak Rebased and implemented requested changes.
Please note that I had to modify scripts/test-all.sh in last commit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

awaiting-changes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

find and grep commands fail (unexpected argument '-n' found)

2 participants

@guillaumedeslandes @pszymkowiak